fix(openapi/upload): more edge case handling for URL uploads #1348
Conversation
also some better warning cleanup
kanadgupta
changed the title
kanadgupta
added
command:openapi
There was a problem hiding this comment.
the diff in this file is pretty rough apologies 😭 i had to shuffle a bit of logic around to hit all of these edge cases and tried to simplify logic and add comments wherever i could. might be easier to just read the file top-to-bottom to be honest 😮💨
There was a problem hiding this comment.
other than the new tests, the other test refactors are either stricter assertions or minor variable renaming!
src/commands/openapi/upload.ts
Outdated
|
|
||
| const fileExtension = nodePath.extname(filename) || '.json'; // default to .json if no extension is found | ||
| const fileExtension = nodePath.extname(filename); | ||
| const isFileYaml = yamlExtensions.includes(fileExtension); |
There was a problem hiding this comment.
Is there a way to determine if the file is yaml besides reading the file extension? It seems a little odd to coerce it into json if a url doesn't have a file extension and we don't have a slug (as asserted in the test should create a new API definition in ReadMe if there is no path and the file is YAML)
There was a problem hiding this comment.
If there isn't an easy way to do this, I think defaulting to openapi.json is fine
There was a problem hiding this comment.
yeah that's a very good question... so oas-normalize is what processes this file (e.g., fetches the URL, converts the file to JSON, validates/bundles it, etc.) and there's a bit of a limitation with URL uploads where we only delineate the file type for local files and not for URLs (see the screenshot of the oas-normalize type parameter below and note how string-yaml and string-json are a thing, but it's just url and not url-json/url-yaml).
my thinking is that if someone passes in a URL and the URL end with a file extension and the user is not passing a slug, we can safely just fallback to assuming it's JSON and call it a day. if they feel otherwise, they can very easily set a slug 🤷🏽 i'm open to fixing this at an oas-normalize level if this becomes a problem that customers complain about, but my hope is that it's an edge case we'll rarely run into!
There was a problem hiding this comment.
you did inspire me to revisit this and i made a small tweak in 59b5637 that should be fairly harmless!
unlikely we'll hit this since this really only affects local YAML files that do not have a file extension... but sure why not!
## [10.5.4-next.1](v10.5.3...v10.5.4-next.1) (2025-10-08) ### Bug Fixes * **openapi/upload:** more edge case handling for URL uploads ([#1348](#1348)) ([76e60b3](76e60b3)) [skip ci]
|
🎉 This PR is included in version 10.5.4-next.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
🧰 Changes
expanding on the work from #1344, this PR tweaks a bunch of logic / DX improvements to better account for the following
rdme openapi uploadedge cases:rdme openapi upload https://example.com --custom-slug.json) will now properly read that file and set that slugrdme openapi upload https://example.com --custom-slug.yml) will now properly read that file and set that slug--slugpassed (e.g.,rdme openapi upload https://example.com) will now default toopenapi.jsonand emit a warning about this behavior--slugpassed (e.g.,rdme openapi upload https://example.com/some-path) will now properly handle JSON/YAML and set the slug to that path (e.g.,some-path.json). [applicable for both creates and updates]🧬 QA & Testing
honestly the more test cases i added, the more rework i realized this logic required 😮💨 the fact that they're passing brings me a lot of comfort here lol